@claude can you review this PR? Note that your review should cover the scope of this PR only. If you spot things unrelated to this PR, feel free to bring them up and we'll consider them for new issues. I want you to think about possible ways that the implementation could be improved. Including ways it could be re-written completely using different abstractions. I want critical comments, do feel comfortable to give me the hard truths.

Claude finished @danbraunai-goodfire's task —— View job

PR Review: App updates and backwards transformer ci functions

Review Scope: 288 commits, 100 files changed, focusing on app updates and transformer CI functions

Todo List

• Activate virtual environment and setup
• Analyze major architectural changes in the app
• Review new abstractions and components
• Examine backend API changes
• Review frontend component refactoring
• Analyze config changes across experiments
• Provide critical feedback on implementation approach
• Suggest alternative architectures and improvements

Critical Review & Hard Truths

🔥 Major Architectural Problems

1. Backend Compute Layer is a God Object (spd/app/backend/compute.py:1-827)

• Problem: 827 lines doing gradient computation, edge calculation, intervention logic, AND result formatting
• Anti-pattern: Multiple responsibilities violating SRP
• Rewrite suggestion:

  # Split into focused modules:
  spd/app/backend/compute/
    ├── gradient_engine.py     # Core autograd operations
    ├── attribution_builder.py # Edge construction logic  
    ├── intervention_runner.py # Forward passes with masks
    └── result_formatters.py   # Output transformation

2. Database Schema Fighting the Type System (spd/app/backend/database.py)

• Problem: StoredGraph uses discriminated unions in Python, but database still has nullable columns for all possible fields
• Result: Invalid states are representable at DB level despite type safety in Python
• Solution: Either use proper table inheritance OR embrace the union and validate constraints in application code, don't half-ass both approaches

3. Frontend State Management is Chaotic (PromptAttributionsTab.svelte:565+ lines)

• Problem: No state machine - implicit transitions everywhere
• Examples:
  • TabViewState + GraphComputeState + ComposerState can be in invalid combinations
  • UI can be in "loading" state while also in "draft" mode
• Rewrite suggestion: Use XState or explicit state machine pattern. Current approach doesn't scale.

🚨 Code Quality Issues

4. Layer Aliasing Global State (spd/app/frontend/src/lib/layerAliasing.ts:57)

let cachedArchitecture: Architecture | null = null;

• Problem: Global mutable cache that can become stale, not reset between contexts
• Solution: Pass architecture context down explicitly or use a proper store

5. Bulk API Endpoint Design (spd/app/backend/routers/component_data.py)

• Good: Eliminated GIL contention by batching requests
• Bad: Still sequential execution inside the handler - missed opportunity for true parallelism
• Alternative: Use asyncio.gather() to run the three data fetches concurrently

6. Configuration Explosion (spd/configs.py)

• Added complex global_reverse_residual CI config with 7+ required fields and complex validation
• Problem: Config complexity is growing faster than the value it provides
• Question: Why not use dataclasses with defaults instead of elaborate Pydantic validation?

💡 Missed Opportunities

7. TypeScript Discriminated Unions Done Halfway

// Current: Can still represent invalid states
type OptimizeConfigDraft = {
    loss: LossConfigDraft;  // Could be invalid
    // ...
}

// Better: Make invalid states unrepresentable  
type OptimizeState = 
  | { status: "draft"; config: Partial<OptimizeConfig> }
  | { status: "valid"; config: ValidOptimizeConfig }
  | { status: "computing"; config: ValidOptimizeConfig }

8. Database Caching Strategy

• Current: Ad-hoc SQLite caching with complex uniqueness constraints
• Alternative: Consider Redis/in-memory cache for computed graphs + persistent storage for configuration only
• Benefit: Simpler schema, better performance for large graphs

🎯 Specific Technical Issues

9. Performance Anti-patterns:

• compute_prompt_attributions() does full gradient computation even for display-only graphs
• useComponentData and useComponentDataExpectCached are nearly identical - code duplication
• Frontend doesn't virtualize large lists (could be 1000+ components)

10. Error Handling Inconsistency:

• Some places use assert for validation (good, fail-fast)
• Others use try/catch with graceful degradation (bad for research code)
• Pick one approach: Research code should fail fast, not silently degrade

🔨 Complete Rewrite Suggestions

Alternative 1: Event-Driven Architecture
Replace current request/response with events:

# Commands
@dataclass
class ComputeAttributionsCommand:
    prompt_id: int
    optimization_params: OptimParams | None

# Events  
@dataclass
class AttributionsComputed:
    prompt_id: int
    graph_data: GraphData

Alternative 2: Functional Core + Imperative Shell
Move all computation to pure functions, keep I/O at edges:

def compute_attributions(model: Model, tokens: Tensor, config: Config) -> Graph:
    # Pure function - no side effects, easy to test
    pass

class AttributionService:
    # Imperative shell - handles caching, DB, progress callbacks
    pass

🚧 Breaking Changes Needed

The current architecture is trying to be too general. Consider:

1. Remove the three graph types - just have "computed graphs" with optional optimization metadata
2. Eliminate the database - store graphs as JSON files, much simpler
3. Split the mega-components - PromptAttributionsTab should be 4-5 smaller components

Bottom Line: This codebase is evolving from research prototype to production-like complexity without the discipline. Either embrace the complexity with proper abstractions, or ruthlessly simplify back to research-code simplicity.

The current middle ground is the worst of both worlds.